Solution: Jordi Arnau & Eunyoung Kim#4
Solution: Jordi Arnau & Eunyoung Kim#4solaz0824 wants to merge 46 commits intoassembler-institute:mainfrom
Conversation
danilucaci
left a comment
danilucaci
left a comment
There was a problem hiding this comment.
Muy buen proyecto! 👏🏻👏🏻 Está muy bien logrado incluso con extras. Felicidades!
|
|
||
| if (!data) { | ||
| return { | ||
| id: Math.random() * 1000, |
There was a problem hiding this comment.
Es más recomendable usar una librería como uuid para generar ids ya que son más seguros.
| body.dark { | ||
| background-color: $mainDark; | ||
| .btn { | ||
| path { | ||
| fill: $mainWhite; | ||
| } | ||
| } | ||
| .mainScreen { | ||
| .upperScreen { | ||
| background-image: linear-gradient(rgba(0, 0, 0, 0.5), rgba(0, 0, 0, 0.5)), | ||
| url("../src/img/background.jpg"); | ||
| .addTodo { | ||
| form { | ||
| input { | ||
| background-color: $mainDark; | ||
| color: $mainWhite; | ||
| } | ||
| .addBtn { | ||
| background-color: #2b425b; | ||
| } | ||
| } | ||
| } | ||
| .TodoList { | ||
| background-color: rgb(3, 17, 12); | ||
| color: $mainWhite; | ||
| .footerStyle { | ||
| .links { | ||
| color: #ebd2e3; | ||
| } | ||
| .filtered { | ||
| color: $mainPurple; |
There was a problem hiding this comment.
Es mejor tener clases menos anidades para evitar que haya una especificidad muy elevada. Una forma muy simple de conseguirlo sería tener todas las clases al mismo nivel y no definidas de otras clases.
There was a problem hiding this comment.
body.dark {
background-color: $mainDark;
.btn {
path {
fill: $mainWhite;
}
}
.upperScreen {
background-image: linear-gradient(rgba(0, 0, 0, 0.5), rgba(0, 0, 0, 0.5)),
url("../src/img/background.jpg");
}
.addTodo {
input {
background-color: $mainDark;
color: $mainWhite;
}
.addBtn {
background-color: #2b425b;
}
}
.TodoList {
background-color: rgb(3, 17, 12);
color: $mainWhite;
.links {
color: #ebd2e3;
}
.filtered {
color: $mainPurple;
}
.footerBtn {
color: $mainWhite;
}
.todo {
border-bottom: 1px solid rgba(61, 1, 56, 0.445) !important;
background: rgb(2, 0, 36);
background: linear-gradient(
90deg,
rgba(2, 0, 36, 1) 0%,
rgba(39, 29, 50, 1) 35%,
$mainPurple 100%
);
color: $mainWhite;
button {
svg {
fill: $mainWhite;
}
}
}
}
.roundedLabel {
border-color: rgba(255, 255, 255, 0.15);
}
}| <input | ||
| type="text" | ||
| className="form-control" | ||
| todo={todo} | ||
| placeholder="Add To Do" | ||
| onChange={handleChange} | ||
| value={todo} | ||
| required | ||
| /> |
There was a problem hiding this comment.
Los inputs deberían tener siempre un label asignado con un text que describa el input. Si no queremos que se vea visualmente el label podemos usar una convención muy común que usa clases llamadas .sr-only que lo que hace es esconder visualmente el elemento en la página pero si que está renderizado para los lectores de pantalla y el navegador.
.sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
border: 0;
}| if (theme === lightTheme || theme === darkTheme) { | ||
| body.classList.add(theme); | ||
| } else { | ||
| body.classList.add(lightTheme); | ||
| } |
There was a problem hiding this comment.
Muy bien implementado el cambio de oscuro a claro. 👏🏻👏🏻
| .mainScreen { | ||
| width: 100%; | ||
| height: 100%; | ||
| font-family: "Josefin Sans", sans-serif; | ||
| .upperScreen { | ||
| background-image: linear-gradient( | ||
| to right, | ||
| rgb(124, 50, 131), | ||
| rgb(64, 57, 128) | ||
| ), | ||
| url("../../../img/background.jpg"); | ||
| background-blend-mode: screen; | ||
| background-repeat: no-repeat; | ||
| background-position: center; | ||
| height: 300px; | ||
| width: 100%; | ||
| display: flex; | ||
| justify-content: center; | ||
| .middleScreen { | ||
| margin-top: 50px; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Es más recomendable tener una estructura de estilos más plana, sin que estén anidados.
.mainScreen {
width: 100%;
height: 100%;
font-family: "Josefin Sans", sans-serif;
}
.upperScreen {
background-image: linear-gradient(
to right,
rgb(124, 50, 131),
rgb(64, 57, 128)
),
url("../../../img/background.jpg");
background-blend-mode: screen;
background-repeat: no-repeat;
background-position: center;
height: 300px;
width: 100%;
display: flex;
justify-content: center;
}
.middleScreen {
margin-top: 50px;
}|
|
||
| const activeList = todoList.filter((todo) => todo.completed === false); | ||
|
|
||
| if (location.pathname === "/active" && activeList.length > 0) { |
There was a problem hiding this comment.
Sería más recomendable pasar la lista de todos ya filtrados a cada página sin tener que comprobar el location.pathname dentro de este componente.
| type="button" | ||
| className={theme === "dark" ? clickedClass : ""} | ||
| id="darkMode" | ||
| onClick={(e) => switchTheme(e)} |
There was a problem hiding this comment.
No need to use the arrow function. Simply onClick={switchTheme} will work
4 participants
No description provided.